refactor: update flow architecture improvements - #454
Merged
Conversation
- Rename AppName → UpdateAppName across all config types - Rename UpgradePath → UpdatePath for consistency - Convert Format from class with const string to enum - Add UpdatePath config for flexible upgrade directory - Simplify DownloadPlanBuilder: no cross-version/chain distinction - Fix temp directory lifecycle: per-zip deletion, last-process cleanup - Remove built-in SignalR Hub integration from Bootstrap - Redesign SilentPollOrchestrator to follow standard AppType split - Add LaunchClientAfterUpdate config flag - Fix OSS flow path resolution and zip naming - Unify parallelism defaults to 2 Closes #453 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs a broad refactor of GeneralUpdate.Core’s update-flow architecture, modernizing configuration naming/type-safety, improving lifecycle handoff (Client → Upgrade), and removing built-in SignalR hub integration.
Changes:
- Renames config fields (
AppName→UpdateAppName,UpgradePath→UpdatePath) and introducesLaunchClientAfterUpdateflow control. - Replaces string-based compression format handling with
enum Format+ToExtension()usage throughout pipeline/download code. - Redesigns silent mode and temp-directory lifecycle to avoid breaking multi-process handoff, and simplifies download plan logic to “server decides, client follows”.
Reviewed changes
Copilot reviewed 73 out of 73 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/UpgradeTest/UpgradeTest.csproj | Adds a new net10 console test app project for upgrade-role runs. |
| tests/UpgradeTest/Program.cs | Adds a runnable upgrade test harness using bootstrap + hooks + event listeners. |
| tests/CoreTest/Strategy/StrategyCreationTests.cs | Updates strategy tests for UpdateAppName renaming and OSS start-app behavior. |
| tests/CoreTest/Silent/SilentPollOrchestratorTests.cs | Updates silent polling tests for removal of AutoInstall. |
| tests/CoreTest/Shared/ConfiginfoBuilderTests.cs | Updates ConfiginfoBuilder tests for renamed UpdateAppName semantics (also writes a test config JSON file). |
| tests/CoreTest/Pipeline/CompressMiddlewareTests.cs | Updates pipeline context Format to be strongly-typed Format enum. |
| tests/CoreTest/Ipc/ProcessInfoProviderTests.cs | Updates IPC tests for new ProcessInfo constructor parameter(s). |
| tests/CoreTest/Ipc/IpcEncryptionTests.cs | Updates IPC encryption tests for updated ProcessInfo shape/ctor. |
| tests/CoreTest/Integration/OssIntegrationTests.cs | Updates OSS integration tests for UpdateAppName. |
| tests/CoreTest/Hooks/HooksIntegrationTests.cs | Updates hook integration tests for UpdateContext.UpdateAppName. |
| tests/CoreTest/FileSystem/BlackListDefaultsTests.cs | Updates blacklist tests to use Format.Zip.ToExtension(). |
| tests/CoreTest/Download/PacketDTOTests.cs | Removes tests for deleted PacketDTO types. |
| tests/CoreTest/Download/DownloadPlanBuilderTests.cs | Removes PacketDTO mapping test; aligns plan behavior to new builder semantics. |
| tests/CoreTest/Configuration/UpdateOptionsStaticTests.cs | Updates options tests for Format enum default; removes Hub/SilentAutoInstall options tests. |
| tests/CoreTest/Configuration/ProcessInfoTests.cs | Updates ProcessInfo tests for new ctor args and fields. |
| tests/CoreTest/Configuration/HubConfigTests.cs | Removes HubConfig tests (Hub integration removed from bootstrap/options). |
| tests/CoreTest/Configuration/ConfigurationModelsTests.cs | Removes HubConfig model tests; retains other model tests. |
| tests/CoreTest/Configuration/ConfigurationMapperTests.cs | Updates mapping tests for UpdateAppName and Format enum. |
| tests/CoreTest/Configuration/ConfigurationMapperExtendedTests.cs | Updates extended mapping tests for Format enum and renamed config fields. |
| tests/CoreTest/Configuration/ConfiginfoTests.cs | Updates validation tests for UpdateAppName rename. |
| tests/CoreTest/Configuration/ConfiginfoBuilderTests.cs | Updates builder tests for SetUpgradeAppName and renamed error messages. |
| tests/CoreTest/Configuration/BaseConfigInfoTests.cs | Updates base-config tests for UpdateAppName default and property sets. |
| tests/CoreTest/Compress/CompressProviderTests.cs | Updates compress tests for Format enum and unknown-format behavior. |
| tests/CoreTest/Bootstrap/ParameterMatrixAndEventTests.cs | Updates bootstrap matrix tests for renamed config fields. |
| tests/CoreTest/Bootstrap/ClientUpgradeIntegrationTests.cs | Updates integration tests for renamed config fields and mapping behavior. |
| tests/CoreTest/Bootstrap/BootstrapHooksAndExtensionsTests.cs | Updates UpdateReport tests for new Spacestation-compatible report shape. |
| tests/CoreTest/Bootstrap/BootstrapFullParameterMatrixTests.cs | Updates full parameter matrix for enum Format, removes Hub/SilentAutoInstall, updates stubs for new interfaces. |
| tests/ClientTest/Program.cs | Adds a runnable client test harness including UpdatePath + UpdateAppName usage. |
| tests/ClientTest/ClientTest.csproj | Adds a new net10 console test app project for client-role runs. |
| src/c#/GeneralUpdate.slnx | Adds new ClientTest/UpgradeTest projects to the solution. |
| src/c#/GeneralUpdate.Core/Strategy/WindowsStrategy.cs | Refactors app launch to use LaunchAppName + UseUpdatePath, adds conditional Bowl launch. |
| src/c#/GeneralUpdate.Core/Strategy/UpgradeUpdateStrategy.cs | Refactors upgrade-role orchestration; adds LaunchClientAfterUpdate gating and pending differ/pipeline injection. |
| src/c#/GeneralUpdate.Core/Strategy/OSSUpdateStrategy.cs | Refactors OSS flow to use InstallPath/UpdatePath, new DownloadSourceResult, and updated zip naming. |
| src/c#/GeneralUpdate.Core/Strategy/MacStrategy.cs | Refactors launch logic to use LaunchAppName + ResolveAppPath. |
| src/c#/GeneralUpdate.Core/Strategy/LinuxStrategy.cs | Refactors launch logic to use LaunchAppName + ResolveAppPath. |
| src/c#/GeneralUpdate.Core/Strategy/IStrategy.cs | Adds required Hooks and Reporter properties to strategies. |
| src/c#/GeneralUpdate.Core/Strategy/ClientUpdateStrategy.cs | Reworks client workflow: uses server-provided per-side flags, scenario dispatch, IPC handoff, and OS-strategy-based launcher. |
| src/c#/GeneralUpdate.Core/Strategy/AbstractStrategy.cs | Adds UpdatePath-aware target resolution and safer temp lifecycle cleanup (per-zip deletion + empty-dir cleanup). |
| src/c#/GeneralUpdate.Core/Silent/SilentPollOrchestrator.cs | Redesigns silent mode: background download + split apply/defer logic + ProcessExit handoff + LaunchClientAfterUpdate. |
| src/c#/GeneralUpdate.Core/Pipeline/DiffPipelineOptions.cs | Changes default parallelism from CPU-count to 2. |
| src/c#/GeneralUpdate.Core/Pipeline/DiffPipelineBuilder.cs | Changes default builder parallelism and docs to 2. |
| src/c#/GeneralUpdate.Core/Pipeline/CompressMiddleware.cs | Reads strongly-typed Format from pipeline context. |
| src/c#/GeneralUpdate.Core/Network/VersionService.cs | Updates request/response parameter casing and adds appKey/upgradeMode fields to validate calls. |
| src/c#/GeneralUpdate.Core/JsonContext/HttpParameterJsonContext.cs | Removes PacketDTO serialization registrations (PacketDTO deleted). |
| src/c#/GeneralUpdate.Core/Ipc/IProcessInfoProvider.cs | Makes encrypted-file IPC deterministic path-based and deletes file after successful read. |
| src/c#/GeneralUpdate.Core/Hooks/IUpdateHooks.cs | Renames UpdateContext field to UpdateAppName and updates Unix permission hook usage. |
| src/c#/GeneralUpdate.Core/FileSystem/BlackListDefaults.cs | Updates default blacklisted formats to use Format.Zip.ToExtension(). |
| src/c#/GeneralUpdate.Core/Download/Sources/OssDownloadSource.cs | Changes IDownloadSource contract to return DownloadSourceResult. |
| src/c#/GeneralUpdate.Core/Download/Sources/HubDownloadSource.cs | Removes built-in SignalR hub download source implementation. |
| src/c#/GeneralUpdate.Core/Download/Sources/HttpDownloadSource.cs | Returns DownloadSourceResult, dedupes assets by URL, maps additional asset metadata (auth, recordId, etc.). |
| src/c#/GeneralUpdate.Core/Download/Reporting/IUpdateReporter.cs | Redesigns reporting to Spacestation-compatible (recordId, status, type) payload. |
| src/c#/GeneralUpdate.Core/Download/Orchestrators/DefaultDownloadOrchestrator.cs | Ensures filename extension matches configured Format when asset.Name is provided. |
| src/c#/GeneralUpdate.Core/Download/Models/DownloadSourceResult.cs | Adds new source result type carrying assets + per-side update flags. |
| src/c#/GeneralUpdate.Core/Download/Models/DownloadOrchestratorOptions.cs | Unifies default concurrency to 2 and adds Format to orchestrator options. |
| src/c#/GeneralUpdate.Core/Download/Models/DownloadAsset.cs | Extends asset model with recordId/auth fields and additional metadata. |
| src/c#/GeneralUpdate.Core/Download/Executors/HttpDownloadExecutor.cs | Adds per-asset authorization header support (scheme/token). |
| src/c#/GeneralUpdate.Core/Download/DownloadPlanBuilder.cs | Simplifies plan building: removes cross-version vs chain distinction; filters/sorts by version + min compatibility. |
| src/c#/GeneralUpdate.Core/Download/Abstractions/PacketDTO.cs | Removes legacy PacketDTO and related records. |
| src/c#/GeneralUpdate.Core/Download/Abstractions/IDownloadSource.cs | Changes IDownloadSource contract to return DownloadSourceResult. |
| src/c#/GeneralUpdate.Core/Configuration/VersionInfo.cs | Adds additional server-mapped fields (cross-version flags, freeze, etc.). |
| src/c#/GeneralUpdate.Core/Configuration/UpdateOptions.cs | Changes Format option to UpdateOption<Format> and removes Hub/SilentAutoInstall options. |
| src/c#/GeneralUpdate.Core/Configuration/ProcessInfo.cs | Adds TempPath/UpdatePath/LaunchClientAfterUpdate to IPC model and updates JSON property naming. |
| src/c#/GeneralUpdate.Core/Configuration/GlobalConfigInfoOSS.cs | Renames JSON-mapped update app name field for OSS config model. |
| src/c#/GeneralUpdate.Core/Configuration/GlobalConfigInfo.cs | Changes Format to enum, adds LaunchClientAfterUpdate, updates defaults (e.g., concurrency=2). |
| src/c#/GeneralUpdate.Core/Configuration/Format.cs | Replaces string constants with enum Format + extension helper. |
| src/c#/GeneralUpdate.Core/Configuration/ConfigurationMapper.cs | Updates mapping for renamed fields, enum Format, new IPC fields (TempPath/UpdatePath/LaunchClientAfterUpdate). |
| src/c#/GeneralUpdate.Core/Configuration/ConfiginfoBuilder.cs | Renames builder method to SetUpgradeAppName and maps to UpdateAppName. |
| src/c#/GeneralUpdate.Core/Configuration/Configinfo.cs | Updates validation to require UpdateAppName instead of AppName. |
| src/c#/GeneralUpdate.Core/Configuration/BaseConfigInfo.cs | Renames base field to UpdateAppName and adds UpdatePath + AppType. |
| src/c#/GeneralUpdate.Core/Configuration/AbstractBootstrap.cs | Refactors extension registration formatting and adds UpdateReporter injection support paths. |
| src/c#/GeneralUpdate.Core/Compress/CompressProvider.cs | Changes compress/decompress API from string-based format to Format enum. |
| src/c#/GeneralUpdate.Core/Bootstrap/GeneralUpdateBootstrap.cs | Refactors bootstrap wiring, removes hub option wiring, updates runtime options handling, and adds IPC Format parsing. |
| src/c#/DifferentialTest/Pipeline/DiffPipelineOptionsTests.cs | Updates tests for new default parallelism = 2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
26
to
33
| var testConfig = new | ||
| { | ||
| UpdateUrl = TestUpdateUrl, | ||
| Token = TestToken, | ||
| Scheme = TestScheme, | ||
| AppName = "Update.exe", | ||
| UpgradeAppName = "Update.exe", | ||
| MainAppName = "TestApp.exe", | ||
| ClientVersion = "1.0.0", |
Comment on lines
10
to
13
|
|
||
| [JsonPropertyName("AppName")] | ||
| public string AppName { get; set; } | ||
| [JsonPropertyName("UpdateAppName")] | ||
| public string UpgradeAppName { get; set; } | ||
|
|
|
|
||
| var upgradeResp = await VersionService.Validate( | ||
| _updateUrl, _upgradeClientVersion ?? _clientVersion, AppType.Upgrade, | ||
| _updateUrl, _upgradeClientVersion , AppType.Upgrade, |
Comment on lines
272
to
275
| private void OnProcessExit(object? sender, EventArgs e) | ||
| { | ||
| if (Volatile.Read(ref _prepared) != 1 || Interlocked.Exchange(ref _updaterStarted, 1) == 1) return; | ||
|
|
Comment on lines
23
to
28
| /// <summary>Optional hooks for pre/post update callbacks.</summary> | ||
| protected IUpdateHooks? Hooks { get; set; } | ||
| public IUpdateHooks? Hooks { get; set; } | ||
|
|
||
| /// <summary>Optional reporter for update status reporting.</summary> | ||
| protected IUpdateReporter? Reporter { get; set; } | ||
| public IUpdateReporter? Reporter { get; set; } | ||
|
|
Comment on lines
+17
to
+21
| /// <summary>Update event types mapped to Spacestation status codes.</summary> | ||
| public enum UpdateEvent { UpdateStarted = 1, DownloadCompleted = 1, UpdateApplied = 2, UpdateFailed = 3, AppStarted = 2 } | ||
|
|
||
| public record UpdateReport( | ||
| string AppName, | ||
| string FromVersion, | ||
| string? ToVersion, | ||
| UpdateEvent Event, | ||
| AppType AppType, | ||
| DateTimeOffset Timestamp, | ||
| string? ErrorMessage = null, | ||
| double? DurationMs = null | ||
| ); | ||
| /// <summary>Spacestation-compatible update report: recordId from verification, status (1=updating,2=success,3=failure), type (1=upgrade,2=push).</summary> | ||
| public record UpdateReport(int RecordId, int Status = 1, int Type = 1); |
Comment on lines
21
to
25
| /// <b>Design:</b> Upgrade does NOT validate versions or download packages. | ||
| /// The client has already validated versions, downloaded all packages, and | ||
| /// passed the results via ProcessInfo. Upgrade only applies updates and | ||
| /// starts the main application — zero network. | ||
| /// starts the main application �?zero network. | ||
| /// </remarks> |
Comment on lines
+67
to
+71
| // Apply MainApp updates �?Client already applied Upgrade packages, IPC only has MainApp versions | ||
| if (_configInfo.UpdateVersions?.Count > 0) | ||
| { | ||
| GeneralTracer.Info("UpgradeUpdateStrategy: applying " + _configInfo.UpdateVersions.Count + " MainApp update(s)."); | ||
| GeneralTracer.Info("UpgradeUpdateStrategy: applying " + _configInfo.UpdateVersions.Count + | ||
| " MainApp update(s)."); |
Comment on lines
18
to
24
| /// <summary> | ||
| /// OSS (Object Storage Service) update strategy — client/upgrade split via AppType. | ||
| /// OSS (Object Storage Service) update strategy �?client/upgrade split via AppType. | ||
| /// <list type="bullet"> | ||
| /// <item><see cref="AppType.OSSClient"/> — downloads version config, checks for updates, | ||
| /// <item><see cref="AppType.OSSClient"/> �?downloads version config, checks for updates, | ||
| /// starts the upgrade process, and exits.</item> | ||
| /// <item><see cref="AppType.OSSUpgrade"/> — reads version config, downloads packages from OSS, | ||
| /// <item><see cref="AppType.OSSUpgrade"/> �?reads version config, downloads packages from OSS, | ||
| /// decompresses them, starts the main app, and exits.</item> |
Comment on lines
263
to
270
| private static void DecompressAssets(List<DownloadAsset> assets, string targetPath) | ||
| { | ||
| var encoding = Encoding.GetEncoding(_configInfo?.Encoding?.CodePage ?? Encoding.UTF8.CodePage); | ||
| var encoding = Encoding.UTF8; | ||
| foreach (var asset in assets) | ||
| { | ||
| var zipFilePath = Path.Combine(_appPath, $"{asset.Name}{Format.ZIP}"); | ||
| CompressProvider.Decompress(Format.ZIP, zipFilePath, _appPath, encoding); | ||
| var zipFilePath = Path.Combine(targetPath, asset.Name); | ||
| CompressProvider.Decompress(Format.Zip, zipFilePath, targetPath, encoding); | ||
|
|
- MaxConcurrency default 3→2 in test assertions - DownloadPlanBuilder.Build returns all matching assets (not just cross-version) - CompressMiddleware exception type changed (Format is now enum, not string) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Fix JSON property name in ConfiginfoBuilderTests (UpgradeAppName -> UpdateAppName) - Rename GlobalConfigInfoOSS.UpgradeAppName -> UpdateAppName for consistency - Add null fallback for _upgradeClientVersion in HttpDownloadSource - Only set _prepared when client packages exist in SilentPollOrchestrator - Make AbstractStrategy.Hooks/Reporter non-nullable with NoOp defaults - Rename UpdateEvent -> UpdateStatus with clear member names (Updating/Success/Failure) - Fix garbled characters in UpgradeUpdateStrategy and OSSUpdateStrategy comments - Pass configured encoding to OSS DecompressAssets instead of hardcoded UTF8 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive refactoring of the update flow architecture, configuration naming, and lifecycle management.
Changes
Config Naming
AppName→UpdateAppNameacross all config types (distinguishes upgrade exe from main app)UpgradePath→UpdatePathfor consistencyFormat Type
Formatconverted fromclasswithconst stringtoenum Format { Zip }UpdateOptions.FormatnowUpdateOption<Format>— removes legacy string normalizationFlexible Upgrade Directory
UpdatePathconfig — upgrade exe can live in a subdirectory (e.g.InstallPath/Upgrade/)UseUpdatePathflagUpdatePathwhen configuredDownload Plan
DownloadPlanBuilder— server decides, client followsTemp Directory Lifecycle
Clear(TempPath)— was breaking Client→Upgrade handoffSignalR Hub
HubDownloadSource,PacketDTO,Huboption from BootstrapUpgradeHubService+HubConfigkept for developer DI registrationSilent Mode Redesign
LaunchClientAfterUpdateflagAutoInstall— unified to process exit triggerOSS Flow Fixes
.zipextension bug_appPath→InstallPath)UpdatePathsupportParallelism
2Files
Closes #453